Skip to content

BDMS 114 & Others: add value_reason to observation | update FieldEventContactAssociation | remove geothermal clutter#171

Merged
jirhiker merged 7 commits into
stagingfrom
jab-observation-field-revisions
Oct 3, 2025
Merged

BDMS 114 & Others: add value_reason to observation | update FieldEventContactAssociation | remove geothermal clutter#171
jirhiker merged 7 commits into
stagingfrom
jab-observation-field-revisions

Conversation

@jacob-a-brown

@jacob-a-brown jacob-a-brown commented Oct 2, 2025

Copy link
Copy Markdown
Contributor

Why

This PR addresses the following problem / context:

  • A user needs to record the reason for a NULL water level value using a controlled vocabulary
  • FieldEventContactAssociation is an unclear name
  • The geothermal group needs to be consulted before their data are imported and legacy code is cluttering the code base

How

Implementation summary - the following was changed / added / removed:

  • Changed level_status to value_reason and made it a non-nullable field for all observations. The value_reason field allows users to provide context for a single observation value. This is particularly important for NULL water levels so we can deduce which are NULL because the well is dry (as opposed to other reasons like an obstruction)
  • FieldEventContactAssociation was renamed FieldEventParticipant for clarity
  • legacy geothermal code in the observation table was removed to declutter the code base

Notes

Any special considerations, workarounds, or follow-up work to note?

  • The consensus on when a groundwater level observation is NULL:
    • If anything occurs before an attempt is made to retrieve a groundwater level and it prevents such an action, then a FieldEvent record is made but no Observation or Sample records are made. It is then noted in the FieldEvent record why this happened. A restricted vocabulary could be useful, but there are so many things that can go wrong we thought to just have it noted in the notes field. This could be anything from a flat tire to a locked gate to muddy roads to a destroyed well.
      • If a well is found to be destroyed there should be a workflow to update the StatusHistory, too.
      • A WaterLevels record that indicates a destroyed well will create a FieldEvent record but no sample or observation
    • If anything happens after a sample/observation has been attempted (such as an obstruction, equipment failure, or dry well), then an observation record is made, value is set to NULL, and controlled vocabulary is used in value_reason to indicate why it is NULL. As written above, this is particularly important for water levels so we can deduce if a well is dry or not.

@codecov-commenter

codecov-commenter commented Oct 2, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Files with missing lines Coverage Δ
api/observation.py 100.00% <ø> (ø)
db/contact.py 100.00% <100.00%> (ø)
db/field.py 100.00% <100.00%> (ø)
db/location.py 95.00% <100.00%> (ø)
db/observation.py 100.00% <100.00%> (ø)
db/publication.py 100.00% <100.00%> (ø)
db/sample.py 100.00% <100.00%> (ø)
db/sensor.py 100.00% <100.00%> (ø)
schemas/observation.py 98.41% <100.00%> (ø)
schemas/sample.py 98.30% <100.00%> (ø)
... and 5 more

Comment thread db/contact.py Outdated
Comment thread db/contact.py Outdated
Comment thread db/observation.py Outdated

@TylerAdamMartinez TylerAdamMartinez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great refactor here! 😄

Comment thread api/observation.py

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this file grows, it would make sense to refactor the api/observation.py section into smaller, more focused modules (e.g., api/observation/get.py, api/observation/post.py, etc.) to improve maintainability and clarity.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you suggest that for all routers? Every one has GET, POST, PATCH, and DELETE methods defined in them.

@jirhiker jirhiker merged commit ff5117e into staging Oct 3, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants